-
-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Debugging mode for CPython universal #43
Conversation
Are we sure this is the design we want to use? An alternative design is to implement the debug mode as a wrapper around an existing
|
While I see the long-term point, I have some doubts about the idea now. In "your" version, we need to wrap at least all functions of the API to check all handles. It doesn't exist so far because it is quite some work and it will add maintenance cost. It risks bugs of its own, because it needs to be complete and consistent. And it needs to be explicitly enabled by the user. By comparison this is a few lines of code, and adding the same ones inside PyPy is trivial. |
I see your point. My idea was to autogen all the wrappers, i.e. something like this: HPy ctx_Long_FromLong(HPyContext ctx, long x)
{
HPy res = ctx->original_ctx->ctx_Long_FromLong(ctx, x);
ctx->_record_new_handle(res);
return res;
} If we do it this way, the only maintenance cost it adds is the autogen code (which, btw, it much nicer and easier to do in my WIP |
#define HPY_ASSERT(condition, errargs) \ | ||
do { \ | ||
if (!(condition)) \ | ||
_hpy_fatal errargs; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can probably use __VA_ARGS__
to avoid the weird usage pattern in which you have to put parenthesis in the call?
@@ -12,6 +12,7 @@ | |||
'hpy/universal/src/ctx_module.c', | |||
'hpy/universal/src/ctx_meth.c', | |||
'hpy/universal/src/ctx_misc.c', | |||
'hpy/universal/src/debugmode.c', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my idea, the "debug mode" be something different, i.e. a full wrapper around the ctx
which does many extra checks, including potentially expensive ones, and will . So, maybe we should call this file differently?
Redone in PR #69. |
Could be merged as it is, but probably only compiles on Linux. Also, there is no way to disable the extra checks, but they don't cost a lot.